[DREAM-693] Unify and document ExpandableTextComponent - #23328
Conversation
ExpandableTextComponent
Deploying openproject with ⚡ PullPreview
|
8d912e3 to
c4c0187
Compare
There was a problem hiding this comment.
Pull request overview
This PR standardizes truncation/expansion behavior around OpPrimer::ExpandableTextComponent, adds Lookbook documentation/previews for it, and migrates OpenProject::Common::AttributeComponent from a bespoke Stimulus/CSS implementation to the shared truncation controller + component.
Changes:
- Extend the Stimulus
truncationcontroller to support horizontal vs vertical truncation and inline vs dialog-style expansion. - Enhance
OpPrimer::ExpandableTextComponent(API + docs) and add Lookbook previews/spec coverage. - Refactor
OpenProject::Common::AttributeComponentto composeExpandableTextComponent, removing the legacyattributecontroller and styles.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/components/open_project/common/inplace_edit_fields/display_fields/rich_text_area_component_spec.rb | Updates expectations to the unified truncation controller / expander visibility. |
| spec/components/op_primer/expandable_text_component_spec.rb | Expands component specs to cover modes, options, and validation. |
| spec/components/op_primer/expandable_text_component_preview_spec.rb | Adds preview rendering specs (including Lookbook path shim). |
| lookbook/previews/op_primer/expandable_text_component_preview.rb | Adds preview class with multiple scenarios + playground params. |
| lookbook/previews/op_primer/expandable_text_component_preview/default.html.erb | Default horizontal example template. |
| lookbook/previews/op_primer/expandable_text_component_preview/in_table.html.erb | Table layout example template. |
| lookbook/previews/op_primer/expandable_text_component_preview/vertical.html.erb | Vertical line-clamp example template. |
| lookbook/previews/op_primer/expandable_text_component_preview/dialog.html.erb | Dialog expansion example template. |
| lookbook/previews/op_primer/expandable_text_component_preview/playground.html.erb | Param-driven playground template. |
| lookbook/docs/components/expandable-text.md.erb | Adds component documentation and usage guidance. |
| frontend/src/stimulus/controllers/truncation.controller.ts | Adds mode/inline values and vertical truncation + class toggling. |
| frontend/src/stimulus/controllers/truncation.controller.spec.ts | Updates/expands unit tests for new controller behavior. |
| frontend/src/stimulus/controllers/dynamic/attribute.controller.ts | Removes legacy attribute truncation controller. |
| frontend/src/global_styles/content/_text_utils.sass | Adds .expandable-text--expanded to undo line-clamp when expanded. |
| app/components/open_project/common/attribute_component.rb | Drops legacy params and adapts to new expander logic. |
| app/components/open_project/common/attribute_component.html.erb | Replaces bespoke truncation markup with ExpandableTextComponent. |
| app/components/open_project/common/attribute_component.sass | Removes legacy CSS for the old attribute truncation implementation. |
| app/components/op_primer/expandable_text_component.rb | Documents/unifies component API; wires controller values and expander args. |
| app/components/op_primer/expandable_text_component.html.erb | Adds vertical rendering path and uses unified expander arguments. |
| app/components/_index.sass | Removes import of deleted legacy attribute component stylesheet. |
7ae6625 to
376bc80
Compare
376bc80 to
c2f0ee7
Compare
Two controllers solved the same truncation-detection problem independently. Unifies them with orthogonal truncation/expansion axes on `ExpandableTextComponent`. Extends `TruncationController` with `mode` (horizontal/vertical) and `inline` (true/false) values. Vertical detection uses `scrollHeight` vs `clientHeight`, working with `-webkit-line-clamp`. Extends `ExpandableTextComponent` with `truncation:`, `lines:`, `inline:`, and `expander_arguments:` params. Adds `expandable-text--expanded` CSS class for vertical inline expansion. https://community.openproject.org/wp/75274
`AttributeComponent` now uses `ExpandableTextComponent` with `truncation: :vertical, inline: false` instead of managing its own truncation markup. Deletes the now-redundant `AttributeController` Stimulus controller and text-hider overlay. https://community.openproject.org/wp/75274
Adds the ExpandableTextComponent Lookbook preview covering the horizontal, vertical, short-text, in-table, dialog, and playground variants and documenting each truncation mode, plus a spec that smoke-tests every preview renders. https://community.openproject.org/work_packages/75274
c2f0ee7 to
da299b9
Compare
|
Warning Flaky specs
|
In dialog mode the expander opens a modal dialog but did not announce that to assistive technology, and HellipButton's hardcoded `aria-expanded` wrongly implied an in-place disclosure. The button now carries `aria-haspopup="dialog"` and `aria-controls`, and the controller drops `aria-expanded` for the dialog variant (it stays for inline).
For now this component is only used internally. This may change in the future.
Replaces raw CSS attribute selectors: the `aria-label` check uses the `aria:` filter, and the class-free data-attribute checks use `have_element` with the attributes as options.
125d790 to
1a92b29
Compare
ExpandableTextComponentExpandableTextComponent
| } else { | ||
| // The button opens a modal dialog (aria-haspopup="dialog"), so the | ||
| // disclosure-style aria-expanded that HellipButton hardcodes does not apply. | ||
| this.expanderButton.removeAttribute('aria-expanded'); |
There was a problem hiding this comment.
@HDinger @bsatarnejad we should consider making this optional in upstream HellipButton.
HDinger
left a comment
There was a problem hiding this comment.
The lookbook docs are much better readable by now 👍
| <div style="max-width: <%= width %>px;"> | ||
| <%= render(OpPrimer::ExpandableTextComponent.new(direction:, lines:, expansion:)) do |component| %> | ||
| <% if expansion == :dialog %> | ||
| <% component.with_dialog(title: "Full text", size: :large) do |dialog| %> | ||
| <% dialog.with_header(variant: :large) %> | ||
| <% dialog.with_body_content(text) %> | ||
| <% end %> | ||
| <% end %> | ||
| <%= text %> | ||
| <% end %> | ||
| </div> |
There was a problem hiding this comment.
I still have one problem with this preview: When choosing horizontal as the direction, the expansion state still goes to the bottom, which is the same as when chosing vertical. I always expect that things open to the right which does not work because the container has a max-width which cannot grow. This is very confusing and I keep stumbling over it even though I know the component by now..
There was a problem hiding this comment.
@HDinger I agree that this was confusing - and would likely trip up devs implementing to this. horizontal/vertical in this case referred to the truncation direction, not the expansion direction.
I've now renamed direction: :horizontal|:vertical → truncate: :single_line|:multi_line. This should make things much clearer!
| @@ -0,0 +1,11 @@ | |||
| <div style="max-width: <%= width %>px;"> | |||
There was a problem hiding this comment.
What is the benefit of making that configurable? Imho, it just complicates the preview
There was a problem hiding this comment.
This shows the behaviour with different max-widths, which I think is useful:
Screen.Recording.2026-06-17.at.19.00.34.mov
Having said that, I've had issues with the range input not staying in sync with the query params (this might be a Lookbook issue?)
| | `horizontal` (default) | A single line, cut off with an ellipsis (`…`) | Short labels in table cells or list rows — permission names, statuses | | ||
| | `vertical` | The first `lines:` rows, then clamped | Multi-line descriptions or rich-text previews | | ||
|
|
||
| `vertical` takes a `lines:` count (1–6, default 3): |
There was a problem hiding this comment.
This is somehow lost here.. Either you make it a real secton###vertical or add as a notes column to the table.
What do you mean with 1-6? Is the max number of lines 6? What if I need more?
There was a problem hiding this comment.
I thought 6 was an OK number for most use cases. I can bump it to 8 just in case.
The reason for imposing a hard limit is that we generate a modifier class for each number of lines.. so op-vertical-truncate--lines-1 to --lines-6. Obviously we don't want to bloat our CSS with an infinite number line numbers. One option would be to use CSS variables instead, e.g. by setting style="--expandable-text-lines:6" as an attribute.
That would be more scalable, but I know you're also weary of inline styles 😃 .. it is also a different approach to the one used in frontend/src/global_styles/content/_text_utils.sass.
Names the `lines: 3` default `LINES_DEFAULT` for consistency with the existing `DIRECTION_DEFAULT` and `EXPANSION_DEFAULT` constants.
Lookbook strips the full `ComponentPreview` suffix, so the inspect path is `expandable_text`, not `expandable_text_component`.
The `direction: :horizontal|:vertical` option named the truncation axis but read as the expansion direction, so callers expected `:horizontal` to expand rightward. Renames it to `truncate: :single_line|:multi_line`, which describes the collapsed output and decouples it from how the text expands. The Stimulus `mode` value and the `multi_line` preview track the same terms.
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
|
@HDinger I think I've addressed the most important you addressed in the latest review, so I will go ahead and merge as-is. I'm keen to tie up this loose end for 17.6. Obviously I'm still open to iterating on the API and behaviour in the future.. |
Raises the supported `lines:` count from 6 to 8. The limit is hard because each value generates a `--lines-N` CSS modifier class, so the range stays bounded rather than open-ended.
Raises the `lines:` lower bound from 1 to 2. A single clamped line is indistinguishable from `single_line`, so allowing it only invited confusion; one line is `Primer::Beta::Truncate`'s job.
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
Ticket
https://community.openproject.org/wp/DREAM-693
What are you trying to accomplish?
Introduces a reusable
ExpandableTextComponentand collapses the two overlapping truncation Stimulus controllers into a single, privateexpandable-textcontroller.The component truncates block content either horizontally (one line, via
Primer::Beta::Truncate) or vertically (multi-line, via the newOpPrimer::VerticalTruncateComponent), exposing an expander that reveals the full text inline or opens a dialog it owns.AttributeComponentis recomposed on top of it (choosing the truncation direction from itslinescount) and the legacyattribute.controller.tsis removed.Screenshots
the behaviour is best seen ib the new ExpandableText Lookbook page (horizontal, vertical, in-table, dialog, and playground previews).
Horizontal
Inline
Inline - within table cells
Vertical
Inline
Dialog
What approach did you choose and why?
A single Stimulus controller replaces the duplicated logic, so horizontal/vertical truncation and inline/dialog expansion share one code path. The component exposes two orthogonal axes —
direction:(:horizontal/:vertical) andexpansion:(:inline/:dialog) — validated through Primer'sfetch_or_fallback, and is documented in Lookbook.In
:dialogmode the component owns the dialog through adialogslot: it generates the dialogid, wires the expander to open it, and renders a default dialog when no slot is supplied, so callers never hand-setshow_dialog_id. The trigger also advertises the dialog to assistive technology (aria-haspopup/aria-controls);aria-expandedis kept on the inline (disclosure) variant only.The
expandable-textcontroller is private toExpandableTextComponent: it operates on the DOM the component renders (includingPrimer::Beta::Truncate's internal.Truncate-text), so it is not a reusable behavior.The new preview spec registers the Lookbook preview path with a small shim because Lookbook is disabled in the test environment; a cleaner mechanism is left for a follow-up.
Vertical expander alignment. In vertical mode the expander is a flex sibling cross-aligned to the last visible line (
align_items: :flex_end), which keeps-webkit-line-clamptruncation detection simple and robust. A bottom-right wrap-around expander (text flowing around the control on the last line) was considered but deferred — it would mean droppingline-clampfor amax-heightclamp plus either per-mode markup divergence (float-spacer) or reviving the removed absolute-overlay + text-hider machinery. To be discussed in review.Merge checklist